if the command fails, get_hwcomp_log_data is in charge to free any allocated buffer. That means here the function should just return and after osp_show_hwcomp_log just always free the buffer. Note, free has a built in NULL check, so if (ptr) free(ptr) is not necessary anymore.

The function nvme_get_log_page() called in get_hwcomp_log_data() but if failed the call then the allocated log->desc not freed so needed to free by the caller or the error return point.

static int get_hwcomp_log_data(struct nvme_dev *dev, struct hwcomp_log *log)
{
...
	log->desc = calloc(1, args.len);
	if (!log->desc) {
		fprintf(stderr, "error: ocp: calloc: %s\n", strerror(errno));
		return -1;
	}

	args.log = log->desc,
	args.lpo = desc_offset,

#ifdef HWCOMP_DUMMY
	memcpy(log->desc, &hwcomp_dummy[desc_offset], args.len);
#else /* HWCOMP_DUMMY */
	ret = nvme_get_log_page(dev_fd(dev), NVME_LOG_PAGE_PDU_SIZE, &args);
	if (ret) {
		print_info_error("error: ocp: failed to get log page (hwcomp: %02X, ret: %d)\n",
				 OCP_LID_HWCOMP, ret);
		return ret;
	}
#endif /* HWCOMP_DUMMY */

Note, free has a built in NULL check, so if (ptr) free(ptr) is not necessary anymore.

That is right. Thank you.

A well behaving function should not change anything on error. So if a function allocates something and then fails later on the function is responsible to free the allocated resources. On the other hand if the function was successful and returns some resources to the caller, the caller is responsible for the resource now.

This makes the error handling on the caller side way more sane.

In the above code path, the correct thing is

	ret = nvme_get_log_page(dev_fd(dev), NVME_LOG_PAGE_PDU_SIZE, &args);
	if (ret) {
		print_info_error("error: ocp: failed to get log page (hwcomp: %02X, ret: %d)\n",
				 OCP_LID_HWCOMP, ret);
		free(log->desc);
		return ret;
	}

Understood well. Thank you for your advice and update the patch.